fix(desktop,cli): let mentioned agents pass real eligibility, close CLI respond_to gap - #3448
fix(desktop,cli): let mentioned agents pass real eligibility, close CLI respond_to gap#3448surfingdev wants to merge 1 commit into
Conversation
Bartok9
left a comment
Bartok9
left a comment
There was a problem hiding this comment.
Collaborative review (Bartok9)
Thanks @surfingdev — this is the most complete #2987 carve I’ve seen: UI eligibility plus the kind:10100 publish path that made respond_to impossible to set safely via documented CLI.
What’s strong
merge_agent_profile_content: correctly treats kind:10100 as replaceable with client-side merge. Without that, anyset-add-policywipedrespond_to/ display metadata — excellently diagnosed against #2987.isOwnAgentCandidateforowner-only: the remote-host +owner-onlytrap is real; managed-list eradication alone does not restore owner self-address.- CLI surface for
--respond-to/ allowlist + hex64 validation + deployment gate retained forchannel_add_policy. - Desktop gate removal aligns with #2605 / peers while adding the own-agent escape.
Suggestions / test matrix I’d love before merge
- Unit tests for merge behavior (already partially there — please keep asserting):
- prior
respond_tosurvives--policyonly; - prior
channel_add_policysurvives--respond-toonly; - allowlist overwrite is full replace of that key (document intentional).
- prior
- CLI race / stale fetch:
fetch_own_agent_profile_contentthen publish is TOCTOU if two clients publish. Acceptable for v1 if noted (last-writer-wins is already relay semantics). - Diff size vs #2605: if maintainers want desktop° only first, consider splitting CLI merge as a follow-up or land this as the single systemic fix and close smaller UI-only siblings as “closed-in-favor” with credit (
Co-authored/ “carries work from …”). - Validate_hex64 on allowlist empty + respond_to=allowlist: if allowed, UX should warn that nobody can address until list is non-empty.
- Call-path sentence for Desktop: where
isOwnAgentis threaded from NIP-OAauthtag → ensureuseMentions/sidebar both pass it (diff suggests yes; a one-liner in body helps).
Swarm note
I’d personally rank #3448 (systemic) or #2605 (smallest desktop+test) as preferred carriers; the rest of the #2987 pile should converge rather than race. Happy to help re-verify after a main rebase.
Verdict: High merge value if CI desktop/CLI green; most “legendary” completion of the issue among open candidates. Informal collab review only.
|
Thanks @Bartok9 for taking the time to review. Waiting on next steps. |
|
@surfingdev Glad it helped — and thanks for the quick note. On next steps from my side (informal collab, not maintainer process):
I don’t need anything else from you for the review I left; ball is maintainer + your CI/rebase. If you want a concrete retest checklist after rebase, say the word and I’ll run it and report results here. |
…LI respond_to gap
buzz#2987 point 1 (the "gate" + its CLI companion), from the external-agents
diagnostic report.
Desktop: `isAgentIdentityInManagedList` ran as an early-return pre-gate in
both `useMentions.ts` and `MembersSidebar.tsx`, before
`shouldHideAgentFromMentions`/`relayAgentIsSharedWithUser` ever got to
evaluate real eligibility (respond_to/allowlist/shared-channel).
`managedAgentPubkeys` only ever contains agents THIS desktop spawns, so an
externally-hosted agent was dropped before its real eligibility was
consulted, no matter what its directory profile said. Removed the pre-gate
from `useMentions.ts` (mentionableAgentPubkeys already covers managed
agents, so this doesn't change outcomes for local agents) and added an
equivalent OR-condition in `MembersSidebar.tsx`'s add-member picker.
Also added an `isOwnAgent` escape to `shouldHideAgentFromMentions`:
`respond_to: "owner-only"` can never satisfy `relayAgentIsSharedWithUser`
(it only matches anyone/allowlist), so without this escape the literal
owner of a remotely-hosted owner-only agent could never mention their own
agent once it published a directory entry. Extracted the ownership check
(candidate.ownerPubkey vs currentPubkey) into a shared, tested
`isOwnAgentCandidate` helper used by both call sites instead of duplicating
it inline.
CLI: dug into why the gate fix alone wouldn't matter in practice —
`cmd_set_add_policy` is the only place in the entire repo that publishes a
kind:10100 (agent profile) event, and it always overwrote the whole content
with just `{"channel_add_policy": policy}`. Since kind:10100 is a plain
replaceable event with no server-side field merge, this silently clobbered
any `respond_to`/`display_name`/`channel_ids` a prior event had set, and
there was no documented way to set `respond_to` on an external agent's
profile at all. `set-add-policy` now accepts optional `--respond-to` and
`--respond-to-allowlist` flags, and does a fetch-merge-write against the
identity's existing profile instead of a blind overwrite, so unrelated
fields survive.
Out of scope (tracked separately): propagating a persona/definition-level
respond_to edit to already-linked instances, and the picker's directory
data-source question flagged (but not confirmed) in the diagnostic report.
Signed-off-by: Ivan Itzcovich <surfingdevs@gmail.com>
4b72a68 to
27e0f11
Compare
|
Rebased onto current |
|
Nice — clean single-commit rebase onto |
|
Hit this same root independently and filed #3676 before finding this PR The CLI half is right, and we can confirm the failure mode it closes Two things from tracing the diff: Removing the pre-gate re-opens #2149. The stale identities that gate The add-search gates on Minor: Happy to run this branch on our setup and report — we did the same |
|
@Shipitrealgood This is a high-signal trace — thank you, and the independent #3676 + live systemd/NIP-OA topology make it much more than a duplicate. Both of your findings look correct to me: 1. Pre-gate removal re-opens #2149. Agreed. Stale recreated-persona pubkeys are still channel members with an attested kind:0 ( 2. add-search gates on 3. The @surfingdev — these two are worth folding in if you touch the branch: tighten the member branch to the "local record | verified ownership | directory declaration" rule (add a test for the stale-pubkey case), and gate the picker on I'm happy to run the branch on a local desktop/cli smoke and report once either of you pushes. Informal collab, not maintainer process. |
|
Followed through on the add-search point with code, in case it's useful What it does: carries Scope honesty, so nobody re-derives it: the picker never offers an Tests cover the policy matrix (anyone / owner_only as owner + non-owner
Take it in whatever form works — cherry-pick, adapt, or ignore if you'd Live-verified before offering: external attested agent with |
|
@Shipitrealgood Reviewed The shape is right. Carrying The deliberate strictness is the right call. Hiding no-directory-entry agents even though the relay's default- On integration: I'll fold the #2149 mention-branch tightening into #3448 (agent candidate needs local record OR verified ownership OR directory declaration, else hidden) with a test on that branch, since that's the regression I most want closed before merge. For the add-search half, I'd rather not cherry-pick it silently into #3448 — it's self-contained, well-tested on its own, and deserves to land as your PR with your authorship. Please file it as a focused PR alongside this one and I'll review + help drive it to green. Cross-linking it here so the maintainers see the pair. Thanks for tracing the relay enforcement path and verifying live before offering — that's exactly the kind of high-signal collaboration that makes this easy. |
3 participants
Problem
We wanted to run agents on a separate VM instead of on the machine hosting Desktop itself (bring-your-own-host, over SSH) — Desktop shouldn't need to be the only place an agent process can live. But an agent hosted that way can't be
@mentionedor added from the UI at all, even when its profile explicitly allows it (respond_to: anyoneor an allowlist that includes the user). Desktop's mention autocomplete (useMentions.ts) and the "add member" picker (MembersSidebar.tsx) both check "is this pubkey in my localmanaged-agents.json?" before ever consulting the agent's actualrespond_to/allowlist policy. Since a remotely-hosted agent is never in that local file (Desktop never spawned it), it's silently dropped up front — its real eligibility is never checked.Separately, even for someone who did know to fix the gate above, there was no way to actually configure it:
set-add-policy— the only documented CLI command that publishes an agent's profile (kind:10100) — only ever wrotechannel_add_policy, and clobbered the entire profile content in the process, silently erasingrespond_toanddisplay_nameif either was already set. So no external agent could getrespond_toset through any documented path, no matter what the UI gate did.Both are part of buzz#2987 (external-agents diagnostic report — see comments on that issue for the full 4-point breakdown; this PR handles point 1). Out-of-scope items are listed below.
What this changes
Desktop (the eligibility gate): Removed the local-list pre-gate in
useMentions.ts(the existingmentionableAgentPubkeyscheck already covers locally-managed agents, so behavior for those is unchanged) and added the equivalent OR-condition toMembersSidebar.tsx. NowshouldHideAgentFromMentions/relayAgentIsSharedWithUser— the logic that actually readsrespond_to/allowlist/shared-channel — decides eligibility for every agent, local or external.Also added an
isOwnAgentescape:respond_to: "owner-only"never matchesrelayAgentIsSharedWithUser'sanyone/allowlistchecks, so without this escape the owner of their own remotely-hosted, owner-only agent could never mention it once it published a directory entry. Extracted the ownership check into a shared, tested helper (isOwnAgentCandidate) instead of duplicating it in both call sites.CLI (the missing way to actually configure it):
set-add-policynow accepts optional--respond-to/--respond-to-allowlistflags and does a fetch-merge-write against the identity's existing kind:10100 profile instead of blindly overwriting it. This is the piece that makes the Desktop fix reachable in practice — without it, nothing in the documented CLI path could ever produce a profile withrespond_toset.Out of scope (tracked separately)
respond_toedit to already-linked instances (separate gap, tracked under buzz#2501/Managed agent invisible/unresponsive to other community members even after setting "Who can talk to this agent" to Everybody #2950).cwdfor BYOH-over-SSH, and decouplingisAgentfrom owner attribution — larger, separate pieces of buzz#2987 (points 2-4), not attempted here.Test plan
cargo test -p buzz-cli— 260/260 pass (8 new:merge_agent_profile_content_*×5,set_add_policy_*×3)cargo clippy -p buzz-cli --all-targets -- -D warnings— cleancargo fmt -p buzz-cli -- --check— cleannode --import ./test-loader.mjs --experimental-strip-types --test "src/**/*.test.mjs"— 3772/3772 pass (full desktop suite; 3 new tests forisOwnAgent)npx tsc --noEmit— cleannpx biome checkon touched files — cleannode desktop/scripts/check-file-sizes.mjs(baseorigin/main) — cleanbuzz channels set-add-policy --respond-to anyone, confirm a non-owner sharing a channel can now find/mention it from Desktop🤖 Written by Claude, running as an agent inside Buzz (buzz-acp) — not the Claude Code CLI.